Data-bound chart components 8/8: full mark-kind coverage - #469
Data-bound chart components 8/8: full mark-kind coverage#469FarhanAliRaza wants to merge 6 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Greptile SummaryThe PR expands data-bound plans to every standalone mark kind through empty structural probes and adds callable-aware plan addressing. It also adds flat Reflex factories, mixed-shape data support, and an all-kinds demo with browser-smoke coverage.
Confidence Score: 4/5The PR should not merge until callable digests account for behavior-affecting global state or reject reducers whose behavior is not fully addressable. A supported module-level reducer can read mutable or worker-specific global state that is absent from its fingerprint, allowing one plan digest to execute different aggregation behavior across reloads or workers. Files Needing Attention: python/reflex_xy/plan.py
|
| Filename | Overview |
|---|---|
| python/reflex_xy/plan.py | Extends plans to structural probes and named callables, but the Python callable fingerprint does not capture behavior-affecting global values. |
| python/xy/marks.py | Adds empty-data structural-probe exits while retaining real-data validation and aggregation paths. |
| python/reflex_xy/data_vars.py | Removes the global equal-length restriction so individual mark validators can enforce their own coupled shapes. |
| python/reflex_xy/factories.py | Adds flat data-bound factories for the remaining standalone mark kinds. |
| scripts/reflex_ws_smoke.py | Adds /kinds navigation and per-cell canvas paint checks to the existing browser smoke harness. |
| examples/reflex/xy_reflex_demo/xy_reflex_demo.py | Adds conditional data-source switching and a gallery route covering data-bound and static chart kinds. |
Reviews (2): Last reviewed commit: "fix(reflex): zero-row structural plan pr..." | Re-trigger Greptile
Merging this PR will not alter performance
Comparing Footnotes
|
There was a problem hiding this comment.
All reported issues were addressed across 13 files
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
d775fd1 to
362394c
Compare
|
Review addressed in b0437af + 362394c — both hard blocks are redesigned, not patched: Synthetic validation is gone entirely. The fix is the core structural-validation path the review asked for: Callables are content-addressed, not name-addressed. Bound methods are refused outright (your two-reducers repro is pinned: Also: the |
362394c to
53dab90
Compare
There was a problem hiding this comment.
Greptile has paused reviews on this repository — it used its 100 free open-source review credits for this billing period. Reviews resume automatically on September 3. To continue before then, an organization admin can keep reviews running past the free credits — those bill as normal usage.
The data-bound plan tier refused the aggregating marks — box, violin, hexbin, contour, heatmap, stairs, ecdf — because their validators need at least one finite value and the Phase 3 decision judged a synthetic-row probe to be validating against made-up data. Answer that objection with discipline instead of exclusion. Each aggregating kind's channels bind fixed placeholder columns from a recorded table (plan._SYNTHETIC_CHANNELS): finite, positive, strictly increasing, one group for the grouped kinds, a square z grid with matching side coordinates, len+1 bin edges. The values satisfy every value-domain precondition a validator imposes, so a probe failure still indicts the structure and never the placeholders. Extending a kind means recording its minimal contract in that table — never a silent guess (§28 spirit). Landing with it: - Flat factories for the newly probed kinds plus triangle_mesh_chart (always zero-row-safe, just never wired). All 19 standalone mark kinds now have one. - validate_columns drops the shared-length requirement: a data var may carry mixed-length and 2-D columns (stairs edges, heatmap grids). Coupled-shape contracts stay with the mark validators at bind, where the errors name the mark and channels involved. - Module-level named callables in mark props (hexbin's reduce_C_function, np.mean by default) content-address as their import path; lambdas and closures have no stable one and are refused toward a module-level function or @reflex_xy.figure. Plan registration is last-write-wins so a hot reload replaces stale node objects behind an unchanged digest. Specs updated: the revision is recorded in the implementation doc's post-landing section, the options doc's decision record, and reflex-integration.md §3.6 "Kind coverage".
Two additions to the demo app, both covered by test_example_apps.py: - A /kinds page rendering all 19 standalone mark kinds as data-bound flat factories fed by a single @reflex_xy.data var — mixed column lengths and a 2-D grid in one var, which the relaxed column validation now permits — next to the composite kinds (pie, radar, sankey, polar, polar bars, wind rose, facet) on the static tier, since those still take data directly. - §10 on the main page: data=rx.cond(...) picking between two data vars. The cond sits at the data-var level, so one fixed plan swaps between two column sets from state and both branches stay compile-checked against the shared schema.
…er runs empty xy.structural_probe() (spec/api/chart-kind-contract.md 'Structural probe'): while active, a mark whose data channels are all empty validates its configuration — enums, bounds, colormaps, range/level shapes — and contributes no traces, instead of refusing zero rows or aggregating. This is the core seam compile gates need to validate chart structure with no data and no invented data; non-empty channels behave identically in and out of the mode. Each aggregating mark (stairs, ecdf, histogram, box, violin, hexbin, contour, heatmap) orders config validation before its data work and gates its zero-row refusal on the mode; _split_by_positions handles the empty grouping case. Pinned three ways in test_validation_timing.py: every kind compiles empty under the probe, still refuses empty normally, and still raises config errors under the probe.
Review hard-blocked the synthetic-column probe as structurally unsound: a column shared between an aggregating channel and a zero-row channel falsely failed on invented lengths (stairs edges len 9 vs scatter's 0), valid hexbin range=/mincnt= configurations falsely failed on invented values, and large gridsize ran real aggregation at page evaluation. - build_plan now compiles every kind zero-row under the core's new structural_probe() mode; _SYNTHETIC_CHANNELS and the shape table are gone. Config errors still fail reflex run; data-dependent outcomes and real-data shape couplings move (back) to bind. Repro pins: shared-column composition and hexbin range/mincnt/gridsize configs. - Plan callables are content-addressed, not name-addressed: import path + code fingerprint (bytecode/names/nested code/defaults) for pure-Python functions — editing a reducer body changes the digest, so rolling deployments resync instead of diverging; import path + distribution version for C-level callables that resolve back to themselves; bound methods, lambdas, closures, and partials refused. - @reflex_xy.data docstring drops the stale equal-length claim. - /kinds gains browser render coverage: reflex_ws_smoke.py step 7 navigates there and pixel-probes all 26 kind cells (19 data-bound + 7 static composites), each addressable as kind-<name>.
…ingerprint boundary cond_summary floor-clamped bin counts to dodge division by zero, so an empty bin would render a mean of exactly 0 — a false origin point at that x-centre. Empty bins are now dropped (columns simply shorten); with the demo's 1M-point seed the output is unchanged, the promise is now unconditional. reflex-integration.md §3.6 records the callable-fingerprint boundary explicitly: the digest addresses code, never process state — mutable module globals in a reducer are the same purity contract every server-side builder tier carries, and are not content-addressable.
The public-surface pin added in stack/5 (`test_public_surface.py`) requires every name in `reflex_xy.__all__` to carry a static declaration alongside its `__getattr__` route, so consumers get real signatures instead of `Any`. This layer's eight new flat factories (box, contour, ecdf, heatmap, hexbin, stairs, triangle_mesh, violin) and the newly curated `triangle_mesh` node re-export were added to `_EXPORTS`/`_XY_REEXPORTS` only, so they typed as missing. Restate them in the `TYPE_CHECKING` block.
53dab90 to
ce04ec4
Compare
|
Rebased onto the current The rebase did surface one real failure, so there is one new commit on top that was not in the reviewed history:
Worth noting because it means the old green CI here was green against a lineage that did not yet contain that pin, not because the surface was complete. Local verification on the rebased branch: Generated by Claude Code |
Stacked on #467 (PR 8 of the reflex component-API stack). Review only the two commits here; everything below them is under review in #461–#467.
What this changes
The data-bound plan tier shipped without the aggregating marks — box, violin, hexbin, contour, heatmap, stairs, ecdf. The Phase 3 decision excluded them because their validators need at least one finite value, and a synthetic-row probe "would validate against made-up data". This lifts that exclusion.
The objection is answered by discipline instead of exclusion. Each aggregating kind's channels bind fixed placeholder columns from a recorded table (
plan._SYNTHETIC_CHANNELS) — finite, positive, strictly increasing, one group for the grouped kinds, a square z grid with matching side coordinates,len+1bin edges. Those values satisfy every value-domain precondition a validator imposes, so a probe failure still indicts the structure, never the placeholders. Extending a kind means recording its minimal contract in that table rather than guessing silently (§28 spirit).Riding along:
triangle_mesh_chart(always zero-row-safe, just never wired). All 19 standalone mark kinds now have one.validate_columnsdrops the shared-length requirement. A data var may carry mixed-length and 2-D columns — a stairs mark'slen+1edges or a heatmap grid beside ordinary row columns. Coupled-shape contracts stay with the mark validators at bind, where the errors name the mark and channels involved.reduce_C_function,np.meanby default) serializes as its import path. Lambdas and closures have no stable address and are refused toward a module-level function or@reflex_xy.figure. Plan registration becomes last-write-wins, so a hot reload replaces stale node objects behind an unchanged digest./kindspage rendering every kind (data-bound flat factories for the 19 marks; composite kinds — pie, radar, sankey, polar, wind rose, facet — on the static tier, since those still take data directly), and §10 on the main page showingdata=rx.cond(...)swapping two data vars under one fixed plan.Spec
Recorded in three places rather than edited in place, so the reversal is legible: the implementation doc's post-landing revision section, the options doc's §8 decision record, and reflex-integration.md §3.6 "Kind coverage (recorded decision, revised 2026-08)".
Verification
uv run pytest tests/reflex_adapter tests/test_validation_timing.py tests/test_example_apps.py— 261 passed, 1 skipped (250 on #467). New pins: the shape table and per-kind plan builds (test_plan.py), the full 19-kind flat table (test_factories.py),SHAPED_ROW_CHARTS— the xy-level half of the contract (test_validation_timing.py), and the/kindspage composing with mixed-length + 2-D columns (test_example_apps.py).pre-commit run --all-files,ruff check,ruff format --check,ty checkall clean.Not run: the browser E2E (
scripts/reflex_ws_smoke.py) against the/kindspage. Worth doing before this merges — the probe proves the plans compile, not that all 19 kinds render.